sequenceDiagram
  participant Repository
  participant LanguageDetector
  participant LintSuppressionAssessor
  participant Git
  participant Finding
  Repository->>LanguageDetector: detect tracked repository languages
  LanguageDetector-->>LintSuppressionAssessor: return applicable languages
  LintSuppressionAssessor->>Git: list tracked source files
  Git-->>LintSuppressionAssessor: return file inventory
  LintSuppressionAssessor->>LintSuppressionAssessor: count suppressions and LOC
  LintSuppressionAssessor->>Finding: return score, evidence, and remediation

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands.}

📈 Test Coverage Report

Coverage calculated from unit tests only

Hi @jwm4, could you PTAL?

@jwm4 and @msu8 I am taking a look at this PR.

🤖 AI-assisted review

Disclosure. This review was produced by an AI agent, not a human reviewer. Every finding below was reproduced empirically against commit 6b3581f before being reported — but treat the analysis as a starting point for human judgment, not as a verdict. Reproduction commands are included so you can verify each claim yourself.

Model	Claude Opus 4.6 (claude-opus-4-6)
Harness	Claude Code CLI
Skill	/pr-review-toolkit:review-pr
Sub-agents	code-reviewer, pr-test-analyzer, silent-failure-hunter, type-design-analyzer, comment-analyzer (5, run independently in parallel)
Scope	git diff main..HEAD @ 6b3581f4f470a5f7feab345ccc98684598468656

Five specialist agents reviewed independently and converged on the same core defect from four different angles. Findings are deduplicated and ranked below.

🔴 Critical — merge blockers

1. The branch breaks CI

tests/unit/test_assessors_structure.py:1513 still asserts default_weight == 0.02, but structure.py:1403 was changed to 0.01 to fund the new attribute.

$ pytest tests/unit/test_assessors_structure.py -q
E   AssertionError: assert 0.01 == 0.02
1 failed, 74 passed

Green on main (75/75), red here. One-line fix.

2. Terraform and Dockerfile support is unreachable dead code

Found independently by all five agents.

LanguageDetector.EXTENSION_MAP has no .tf entry, and detection is suffix-only (path.suffix), so an extensionless Dockerfile can never match. repository.languages therefore can never contain "Terraform" or "Dockerfile" — making the pattern entries at code_quality.py:1961,1963 inert, since both is_applicable() and the scan loop intersect against that dict.

Reproduced end-to-end on a repo containing 20 # tflint-ignore and 3 # hadolint ignore= directives:

detected languages: {'Python': 3}
pass 100.0  '0 suppressions / 3 LOC (0.0/1k)'

This is advertised in three user-facing places that are now false:

• the Attribute.description at code_quality.py:2044 — ships in every generated report
• docs/attributes.md:1508 — "Supported languages: … Terraform … Dockerfile …"
• the tests

The tests certify the broken behavior. test:1324 and :1344 hand-construct languages={"Terraform": 1} / {"Dockerfile": 1} — a state the scanner cannot emit, at a count below minimum_file_threshold = 3. They pass, and prove nothing about production.

3. Markdown/YAML in the denominator inverts the metric

Markdown and YAML are first-class scanned languages. Both are near-universally present (threshold is only 3 files) and both contribute their full line count to total_lines while carrying essentially zero suppressions. The metric is documented as "per 1,000 lines of source code".

Synthetic Go repo at a genuine 100 suppressions/1k lines, plus 24k lines of Markdown docs:

The score flips 0 → 100 purely because the project is documented — the exact inverse of the intended signal. On this repo the dilution is 3x (0.2/1k as shipped vs 0.6/1k Python-only).

Compounding it: code_quality.py:2158 uses len(text.splitlines()), counting blank lines, whereas LanguageDetector.count_total_lines() deliberately excludes them — a further 15–25% denominator inflation. splitlines() also splits on \v \f \x1c \x1d \x1e \x85 
 
; a file with 100 form-feed separators counted 302 lines against 202 real ones. Every one of these errors pushes toward a passing score.

4. Three silent-failure paths under-count the numerator while still reporting pass/100

All reproduced:

• lstat() OSError → continue (:2136-2139). A sparse checkout lists index entries that aren't on disk; every one is silently discarded. A repo at a true 714/1k (maximal fail) reported pass, score 100.0, 0 suppressions / 1 LOC.
• The 1 MB read cap (:2152). A 1.79 MiB big.go with 2000 //nolint past the cap → pass, 0 suppressions / 17,139 LOC (true: 32,001 lines). Large files are precisely where generated/vendored suppressions cluster.
• _count_file_suppressions returns (0,0) on OSError — byte-identical to "a clean empty file". A mode-000 file holding 50 # noqa vanished entirely: absent from the count, the denominator, and the top-offenders list.

Note that test_file_read_is_byte_bounded asserts this silence is correct — it hides 20 real suppressions past a patched cap and asserts status == "pass" and "Total suppressions: 0". It should assert that truncation is disclosed.

For a density metric, a swallowed read is worse than a crash: it produces a falsely good certification, and nobody reads a passing finding's evidence.

5. AGENTS.md Guideline 7 — docs not synced for the two defunded weights

• docs/attributes.md:1153 still documents cyclomatic_complexity as Weight: 2% (now 0.01)
• docs/attributes.md:1330 still documents architectural_boundaries as Weight: 2% (now 0.01)

The diff does touch docs/attributes.md, but misses the two attributes it defunded.

🟡 Important

🔵 Suggestions

• The class docstring's "Other languages always include all files" (:2029) is false — _TEST_DIR_FRAGMENTS is checked before the per-language branch, so tests/foo.sh and spec/values.yaml are excluded. Only the filename-suffix heuristics are language-specific.
• docs/attributes.md:1504 breaks house style — no **ID**/**Tier**/**Weight**/**Category**/**Status** block, no #### Definition / Why It Matters / Measurable Criteria, no **Tools** / **Citations**. Compare ### Threat Model immediately above. The inline-(id, weight) heading format is the convention for the bullet lists, not for full ### sections.
• The example-config comment exclude_tests: false # set true to exclude test files from LOC count describes the wrong effect — the flag skips files entirely (numerator and denominator). Opposite-signed from what someone tuning it would predict. The LintSuppressionOptions docstring gets this right; the YAML comments contradict it.
• _MAX_SUPPRESSION_FILE_BYTES = 1_000_000 # 1 MiB — that's 1 MB. Same in docs/attributes.md:1510. Either say "1 MB" or use 1 << 20.
• default-weights.yaml:12 and :72 still say Tier 3 has 8 attributes; it's 9. (Line 3 was correctly bumped to 30.)
• The changelog block labels this v2.5.0, but pyproject.toml is at 2.50.0 and lint_config_coverage — labeled v2.4.0 — actually shipped in 2.50.0. The version numbering in that block has drifted from real releases; consider keying entries by branch/issue instead.
• pass_per_kloc uses gt=0, forbidding 0.0 — the legitimate strictest policy ("no suppressions tolerated"). ge=0 on pass, gt=0 on fail.
• _get_options unpacks the validated model into tuple[float, float, bool] — two positionally-adjacent floats of identical type, reintroducing the transposition hazard the validator exists to prevent. Return the LintSuppressionOptions instead.
• Consider frozen=True on LintSuppressionOptions: pydantic v2 doesn't re-run mode="after" validators on assignment, and _SUPPRESSION_DEFAULTS is a process-wide shared mutable instance.

✅ Strengths

• The security posture is genuinely strong. git-only discovery with a deliberate no-fallback policy (well-commented at :2113 — exactly the kind of "why" comment that stops a future maintainer from helpfully adding the fallback back), lstat + stat.S_ISREG symlink rejection, bounded reads. Mutation probes confirmed the symlink and gitignore tests each catch their regression, and the symlink test's use of a gitignored target to isolate the behavior is well-judged.
• _GitInventoryUnavailable is the best-designed type in the diff — it cleanly separates "cannot measure" (skipped) from "measured zero" (not_applicable) from "bug" (error), and makes the no-silent-fallback decision greppable.
• Weight arithmetic is correct: 30 attributes summing to exactly 1.0, tiers 58/27/13/2, and the new attribute is properly funded by reductions elsewhere.
• black, isort, ruff check all clean. Line coverage of the new code is ~97%.
• Assessor registration follows the AGENTS.md 4-step checklist correctly.
• The custom two-threshold interpolation instead of calculate_proportional_score() is a defensible deviation — the helper models a single threshold and can't express a pass/fail band.

Recommended sequence

1. Fix the red test ([P0] Create Automated Demo #1) — unblocks CI.
2. Decide Terraform/Dockerfile ([P0] Fix Critical Security & Logic Bugs from Code Review #2): add .tf + a filename rule to EXTENSION_MAP, or strip them from the pattern tables, the Attribute.description, the docs, and the two tests. Then add the guard that would have caught this pre-merge:

   def test_suppression_tables_agree_with_detector():
       assert set(_SUPPRESSION_PATTERNS) == set(_LANG_EXTENSIONS)
       assert set(_SUPPRESSION_PATTERNS) <= set(LanguageDetector.EXTENSION_MAP.values())

3. Fix the denominator ([P0] Bootstrap AgentReady Repository on GitHub #3): exclude Markdown/YAML from LOC (or report per-language), and stop counting blank lines. This is the change that most affects what the assessor actually measures.
4. Make silent skips visible ([P0] Report Header with Repository Metadata #4): thread a scan-health tally (files_missing, files_unreadable, files_truncated) through the walk, and emit it in evidence unconditionally — on pass as well as fail. If the un-scanned ratio exceeds ~10%, downgrade to skipped rather than reporting a density at all. A density computed over an unknown fraction of the codebase isn't a measurement.
5. Derive _LANG_EXTENSIONS from EXTENSION_MAP ([P4] Research Report Generator/Updater Utility #7) — structurally eliminates this entire drift class.
6. Drop assessor_options ([P4] Repomix Integration #8) — re-add it in the PR that first needs it, with a reader, a precedence rule, and a test.
7. Sync the two weight docs ([P0] Improve HTML Report Design (Font Size & Color Scheme) #5), add boundary tests ([P3] Report Schema Versioning #6), re-run.

The security-critical paths here were clearly thought about carefully, and the no-fallback discipline around git ls-files is a good instinct. The gap is that the measurement paths weren't held to the same standard: the language table was written without checking it against the detector, and several tests confirm the implementation's assumptions rather than the behavior users will get.

_{Posted by an AI agent (Claude Opus 4.6 via Claude Code, /pr-review-toolkit:review-pr). Findings are reproducible — please verify before acting, and correct anything that's wrong.}

Hmm, a very suspicious CI fail with 7k lines

Hmm, a very suspicious CI fail with 7k lines

@msu8 ignore that, it was from a bad merge. I overlooked this CI failure from a different PR., I shall post a fix soon and you can rebase on your end.

Got it. Anyway, I think I addressed both your review and the coderabbit stuff in the last two commits.

🎉 This PR is included in version 2.52.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀